Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

Rename Volume => BindMountVolume #273

Merged
merged 1 commit into from
Feb 2, 2018
Merged

Rename Volume => BindMountVolume #273

merged 1 commit into from
Feb 2, 2018

Conversation

cjh1
Copy link
Contributor

@cjh1 cjh1 commented Feb 1, 2018

To be more explicit about the sort of volumes we are dealing with and clear the way for supporting other volume types in the future.

To be more explicit about the sort of volumes we are dealing with and
clear the way for supporting other volume types in the future.
@cjh1 cjh1 requested review from kotfic and zachmullen February 1, 2018 16:48
@cjh1
Copy link
Contributor Author

cjh1 commented Feb 1, 2018

Related to #270

@zachmullen
Copy link
Member

Technically a breaking change, though it doesn't break my use case (I just use VolumePath). I'm fine with it, but I'd like to know what the eventual API will look like -- will BindMountVolume and the real Volume share a base class, or be two totally different things?

@cjh1
Copy link
Contributor Author

cjh1 commented Feb 1, 2018

@zachmullen It is, but as we haven't reached 1.0 I figure it was worth doing now. The eventual API would probably have a Volume base class with sub classes of BindMountVolume and ManagedVolume or something similar.

@zachmullen
Copy link
Member

Ok, thanks.

Copy link
Member

@zachmullen zachmullen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve, but @kotfic 's review should be the authoritative one.

Copy link
Contributor

@kotfic kotfic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this looks good to me. Thanks!

@cjh1 cjh1 merged commit b21cd55 into master Feb 2, 2018
@zachmullen zachmullen deleted the bind-mount branch October 26, 2018 13:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants